OpenSSL: handle SSL_shutdown's errors properly

Origin: upstream, https://code.qt.io/cgit/qt/qtbase.git/commit/?id=8ddffc6ba4f38bb8
Last-Update: 2020-08-19

Do not call SSL_shutdown on a session that is in handshake state (SSL_in_init(s)
returns 1). Also, do not call SSL_shutdown if a session encountered a fatal
error (SSL_ERROR_SYSCALL or SSL_ERROR_SSL was found before). If SSL_shutdown
was unsuccessful (returned code != 1), we have to clear the error(s) it queued.
Unfortunately, SSL_in_init was a macro in OpenSSL 1.0.x. We have to
resolve SSL_state to implement SSL_in_init.

Gbp-Pq: Name CVE-2020-13962.diff

diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp
index 4e9e947263156294ef4c2fc3f522a1db624f3d27..5c9e589ec396d571f4d2c5dbc7f44c261d880b57 100644 (file)
--- a/src/network/ssl/qsslsocket.cpp
+++ b/src/network/ssl/qsslsocket.cpp
@@ -2166,7 +2166,7 @@ void QSslSocketPrivate::init()
     pendingClose = false;
     flushTriggered = false;
     ocspResponses.clear();
-
+    systemOrSslErrorDetected = false;
     // we don't want to clear the ignoreErrorsList, so
     // that it is possible setting it before connecting
 //    ignoreErrorsList.clear();

diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp
index 51510f1c60badd682d8227cd2a14bf9fd3cfba60..855865209bcc72c56680ac7acdc3b6b49d00a40e 100644 (file)
--- a/src/network/ssl/qsslsocket_openssl.cpp
+++ b/src/network/ssl/qsslsocket_openssl.cpp
@@ -648,10 +648,16 @@ bool QSslSocketBackendPrivate::initSslContext()
 void QSslSocketBackendPrivate::destroySslContext()
 {
     if (ssl) {
-        // We do not send a shutdown alert here. Just mark the session as
-        // resumable for qhttpnetworkconnection's "optimization", otherwise
-        // OpenSSL won't start a session resumption.
-        q_SSL_shutdown(ssl);
+        if (!q_SSL_in_init(ssl) && !systemOrSslErrorDetected) {
+            // We do not send a shutdown alert here. Just mark the session as
+            // resumable for qhttpnetworkconnection's "optimization", otherwise
+            // OpenSSL won't start a session resumption.
+            if (q_SSL_shutdown(ssl) != 1) {
+                // Some error may be queued, clear it.
+                const auto errors = getErrorsFromOpenSsl();
+                Q_UNUSED(errors);
+            }
+        }
         q_SSL_free(ssl);
         ssl = nullptr;
     }
@@ -1084,6 +1090,7 @@ void QSslSocketBackendPrivate::transmit()
             case SSL_ERROR_SSL: // error in the SSL library
                 // we do not know exactly what the error is, nor whether we can recover from it,
                 // so just return to prevent an endless loop in the outer "while" statement
+                systemOrSslErrorDetected = true;
                 {
                     const ScopedBool bg(inSetAndEmitError, true);
                     setErrorAndEmit(QAbstractSocket::SslInternalError,
@@ -1681,8 +1688,12 @@ bool QSslSocketBackendPrivate::checkOcspStatus()
 void QSslSocketBackendPrivate::disconnectFromHost()
 {
     if (ssl) {
-        if (!shutdown) {
-            q_SSL_shutdown(ssl);
+        if (!shutdown && !q_SSL_in_init(ssl) && !systemOrSslErrorDetected) {
+            if (q_SSL_shutdown(ssl) != 1) {
+                // Some error may be queued, clear it.
+                const auto errors = getErrorsFromOpenSsl();
+                Q_UNUSED(errors);
+            }
             shutdown = true;
             transmit();
         }

diff --git a/src/network/ssl/qsslsocket_openssl11_symbols_p.h b/src/network/ssl/qsslsocket_openssl11_symbols_p.h
index 0fe0899d4fd9c7b0f8ea866102450429287cf4aa..b7193ad1807272fe380e0d11de5e9e10cf5d5643 100644 (file)
--- a/src/network/ssl/qsslsocket_openssl11_symbols_p.h
+++ b/src/network/ssl/qsslsocket_openssl11_symbols_p.h
@@ -192,4 +192,11 @@ typedef int (*q_SSL_psk_use_session_cb_func_t)(SSL *, const EVP_MD *, const unsi
 }
 void q_SSL_set_psk_use_session_callback(SSL *s, q_SSL_psk_use_session_cb_func_t);
 
+#if OPENSSL_VERSION_NUMBER < 0x10101000L
+// What a mess!
+int q_SSL_in_init(SSL *s);
+#else
+int q_SSL_in_init(const SSL *s);
+#endif // 1.1.1 or 1.1.0
+
 #endif

diff --git a/src/network/ssl/qsslsocket_openssl_symbols.cpp b/src/network/ssl/qsslsocket_openssl_symbols.cpp
index 26336edd3d69b24f5b208e44311561e22457ae58..cbedf96d08aa6fb95e10d400b17c38926fe07792 100644 (file)
--- a/src/network/ssl/qsslsocket_openssl_symbols.cpp
+++ b/src/network/ssl/qsslsocket_openssl_symbols.cpp
@@ -160,6 +160,11 @@ DEFINEFUNC(void, OPENSSL_sk_free, OPENSSL_STACK *a, a, return, DUMMYARG)
 DEFINEFUNC2(void *, OPENSSL_sk_value, OPENSSL_STACK *a, a, int b, b, return nullptr, return)
 DEFINEFUNC(int, SSL_session_reused, SSL *a, a, return 0, return)
 DEFINEFUNC2(unsigned long, SSL_CTX_set_options, SSL_CTX *ctx, ctx, unsigned long op, op, return 0, return)
+#if OPENSSL_VERSION_NUMBER < 0x10101000L
+DEFINEFUNC(int, SSL_in_init, SSL *a, a, return 0, return)
+#else
+DEFINEFUNC(int, SSL_in_init, const SSL *a, a, return 0, return)
+#endif
 #ifdef TLS1_3_VERSION
 DEFINEFUNC2(int, SSL_CTX_set_ciphersuites, SSL_CTX *ctx, ctx, const char *str, str, return 0, return)
 DEFINEFUNC2(void, SSL_set_psk_use_session_callback, SSL *ssl, ssl, q_SSL_psk_use_session_cb_func_t callback, callback, return, DUMMYARG)
@@ -242,6 +247,7 @@ DEFINEFUNC2(void, BIO_set_shutdown, BIO *a, a, int shut, shut, return, DUMMYARG)
 // Functions below are either deprecated or removed in OpenSSL >= 1.1:
 
 DEFINEFUNC(unsigned char *, ASN1_STRING_data, ASN1_STRING *a, a, return nullptr, return)
+DEFINEFUNC(int, SSL_state, const SSL *a, a, return 0, return)
 
 #ifdef SSLEAY_MACROS
 DEFINEFUNC3(void *, ASN1_dup, i2d_of_void *a, a, d2i_of_void *b, b, char *c, c, return nullptr, return)
@@ -971,6 +977,7 @@ bool q_resolveOpenSslSymbols()
 #if QT_CONFIG(opensslv11)
 
     RESOLVEFUNC(OPENSSL_init_ssl)
+    RESOLVEFUNC(SSL_in_init)
     RESOLVEFUNC(OPENSSL_init_crypto)
     RESOLVEFUNC(ASN1_STRING_get0_data)
     RESOLVEFUNC(EVP_CIPHER_CTX_reset)
@@ -1066,6 +1073,7 @@ bool q_resolveOpenSslSymbols()
 #else // !opensslv11
 
     RESOLVEFUNC(ASN1_STRING_data)
+    RESOLVEFUNC(SSL_state)
 
 #ifdef SSLEAY_MACROS
     RESOLVEFUNC(ASN1_dup)

diff --git a/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h b/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h
index f5626d5d16452c43c47ff4a7034bec162ec07768..92841017793c2f313080b4fa440dfaef0dd6745c 100644 (file)
--- a/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h
+++ b/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h
@@ -121,6 +121,8 @@ SSL_CTX *q_SSL_CTX_new(const SSL_METHOD *a);
 
 int q_SSL_library_init();
 void q_SSL_load_error_strings();
+int q_SSL_state(const SSL *a);
+#define q_SSL_in_init(a) (q_SSL_state(a) & SSL_ST_INIT)
 
 #if OPENSSL_VERSION_NUMBER >= 0x10001000L
 int q_SSL_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func, CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);

diff --git a/src/network/ssl/qsslsocket_p.h b/src/network/ssl/qsslsocket_p.h
index daa9be23f4ad363a5dec03fa549abffa0c97bb61..350b1f1fc18dcfb16150b0a968c7ebb75688f447 100644 (file)
--- a/src/network/ssl/qsslsocket_p.h
+++ b/src/network/ssl/qsslsocket_p.h
@@ -208,6 +208,7 @@ protected:
     bool verifyErrorsHaveBeenIgnored();
     bool paused;
     bool flushTriggered;
+    bool systemOrSslErrorDetected = false;
     QVector<QOcspResponse> ocspResponses;
 };